-
Notifications
You must be signed in to change notification settings - Fork 3.1k
refactor: replace lowlevel Server decorators with on_* constructor kwargs #1985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
maxisbey
wants to merge
24
commits into
main
Choose a base branch
from
sketch/lowlevel-server-v2-kwargs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,523
−5,481
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
acea3d7 to
8e1d947
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
maxisbey
commented
Feb 3, 2026
Kludex
reviewed
Feb 3, 2026
7221cca to
7e9e53f
Compare
Kludex
reviewed
Feb 9, 2026
Kludex
reviewed
Feb 9, 2026
…args Replace the decorator-based handler registration on the lowlevel Server with direct on_* keyword arguments on the constructor. Handlers are raw callables with a uniform (ctx, params) -> result signature. - Server constructor takes on_list_tools, on_call_tool, etc. - String-keyed dispatch instead of type-keyed - Remove RequestT generic from Server (transport-specific, not bound at construction) - Delete handler.py and func_inspection.py (no longer needed) - Update ExperimentalHandlers to use callback-based registration - Update MCPServer to pass on_* kwargs via _create_handler_kwargs() - Update migration docs and docstrings
- Fix all migration.md examples to use ServerRequestContext instead of RequestContext - Fix all imports to use 'from mcp.server import Server, ServerRequestContext' - Apply ruff formatting to code examples - Add Server constructor calls to examples that were missing them - Re-export ServerRequestContext from mcp.server.__init__ - Add type hints to TaskResultHandler docstring example
…erver Move handler functions from a dict-returning method to private methods on MCPServer, passed directly to the Server constructor by name. This eliminates the **kwargs unpacking pattern and makes the handler registration explicit.
Allow users to override default task handlers by passing on_get_task, on_task_result, on_list_tasks, and on_cancel_task to enable_tasks(). User-provided handlers are registered first; defaults fill in the rest.
The subscribe capability in ResourcesCapability was hardcoded to False, even when on_subscribe_resource handler was provided. Now dynamically checks whether a subscribe handler is registered. Also applies ruff formatting to experimental.py.
e9df629 to
ca8fd2e
Compare
Migrate test files from the old decorator-based handler registration to the new on_* constructor kwargs pattern. Key changes: - Replace @server.list_tools(), @server.call_tool(), etc. decorators with on_list_tools, on_call_tool, etc. kwargs on Server() - Replace server.request_context access with ctx parameter (first argument to all handlers) - Handlers now receive (ctx, params) and return full result types (e.g. ListToolsResult instead of list[Tool]) - Convert experimental task decorators to enable_tasks() kwargs - Add LifespanContextT default to ServerRequestContext - Widen on_call_tool return type to include CreateTaskResult - Delete redundant tests/shared/test_memory.py - Simplify tests to use Client where possible
Convert the 3 remaining experimental task server test files: - test_integration.py: proper lifespan + Client pattern - test_run_task_flow.py: constructor kwargs + Client pattern - test_server.py: enable_tasks() kwargs + Client pattern All tests use proper typing with ServerRequestContext and constructor kwargs instead of decorators.
- Delete test_func_inspection.py (tested removed create_call_wrapper) - Delete test_lowlevel_input_validation.py and test_lowlevel_output_validation.py (tested jsonschema validation removed from low-level server) - Migrate test_read_resource.py to E2E with Client - Migrate test_129_resource_templates.py to E2E with Client - Migrate test_output_schema_validation.py to constructor kwargs (removed bypass_server_output_validation hack, no longer needed) - Migrate test_ws.py, test_sse.py, test_streamable_http.py from Server subclasses with decorators to standalone handlers with constructor kwargs - Replace self.request_context contextvar with ctx parameter - Replace global _server_lock with typed ServerState lifespan context - Fix import paths for TextContent, ClientRegistrationOptions, RevocationOptions, StreamableHTTPServerTransport
enable_tasks registers default handlers for all task methods, so partial capabilities aren't currently possible. Skipped tests with TODO(maxisbey) to revisit when low-level API supports selectively enabling/disabling individual task capabilities.
…attern - Convert all low-level Server examples from decorator-based handlers to constructor on_* kwargs pattern - Replace server.request_context with ctx parameter passed to handlers - Replace auto-wrapped return values (dict, str, bytes) with explicit result type construction (CallToolResult, ReadResourceResult, etc.) - Update everything-server to use _add_request_handler for subscribe/ unsubscribe/logging handlers (MCPServer doesn't expose these yet) - Update migration docs with detailed section on removed auto-wrapping - Update README snippets
Remove references to automatic content/structured conversion and the four return type options that no longer exist. Low-level handlers now always return CallToolResult directly.
- Remove unused _add_notification_handler from Server - Add pragma no cover for dead dict code path in MCPServer._handle_call_tool - Add E2E test for MCPServer.completion() decorator - Replace unused handle_list_tools stubs with raise NotImplementedError - Add pragma no cover for skipped tests in test_server and test_spec_compliance - Flatten nested completion handler logic to eliminate untaken branches - Fix test_88_random_error to use assert + shared return path - Clean up unused imports (Tool, ToolExecution, TASK_REQUIRED)
Contributor
Author
|
Although CI is all green, I still want to manually test all the examples that were modified. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace the decorator-based handler registration on the lowlevel
Serverwith directon_*keyword arguments on the constructor. Handlers are now raw callables with a uniform(ctx, params) -> resultsignature, dispatched by method string.Supersedes #1968.